Skip to content

feat(demo): completed initial version of animations demo#11876

Closed
parthivrh wants to merge 17 commits intopatternfly:mainfrom
parthivrh:animation-demo-page
Closed

feat(demo): completed initial version of animations demo#11876
parthivrh wants to merge 17 commits intopatternfly:mainfrom
parthivrh:animation-demo-page

Conversation

@parthivrh
Copy link
Copy Markdown
Contributor

What: This demonstration highlights PatternFly's latest animated components, like alerts, navigation, and form validation. Closes #11810

Additional issues:

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Jun 18, 2025


## Demos

This demonstration highlights PatternFly's latest animations. Explore how components like alerts, navigation, and forms use motion to provide clear feedback and improve usability across the platform.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This demonstration highlights PatternFly's latest animations. Explore how components like alerts, navigation, and forms use motion to provide clear feedback and improve usability across the platform.
The following demo highlights the current state of [our ongoing effort to animate PatternFly components](https://github.com/orgs/patternfly/projects/7/views/66).
To see how components like alerts, navigation, and forms can now use motion to provide clear feedback and improve usability, you can explore this demo and interact with various UI elements. We will continue to update this demo as additional animation support is added.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also add a more specific list if you think we want to call out the details. I'll let you decide based on what seems the most useful for this demo

"Currently, this demo includes animations for:

  • Alerts.
  • The notification badge and notification drawer.
  • The hamburger/navigation menu icon.
  • The masthead settings icon.
  • Expandable navigation items.
  • Skeleton loader in a table.
  • Button clicks.
  • Validation failure in forms."

{loading ? (
<SkeletonTable columns={['', ...expandableColumns]} rows={8} />
) : (
<Table aria-label="Collapsible table">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll need to opt in to the expansion animation with hasAnimations on the Table.

};

const detailStatusEvents = (
<Gallery hasGutter style={{ width: '100%', display: 'grid', gridTemplateColumns: 'repeat(3, 1fr)' }}>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you take the inline style off and use the gallery props instead?

@parthivrh parthivrh force-pushed the animation-demo-page branch from 11f8da6 to 96bf1f6 Compare June 23, 2025 20:34
Copy link
Copy Markdown
Contributor

@edonehoo edonehoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content looks good to me!

Copy link
Copy Markdown
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! Just a couple of really nitpicky comments 😉

const handlePasswordChange = (_event, password: string) => {
setPassword(password);
setIsPasswordValid(
password.length > 12 && /[0-9]/.test(password) && /[A-Z]/.test(password) ? 'success' : 'error'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nitpicky 😄 but the text says "at least 12 characters" and this forces you to have more than 12.

Comment on lines +1190 to +1202
const detailStatusEvents = (
<Grid hasGutter>
<GridItem span={4}>
<DetailsCard />
</GridItem>
<GridItem span={4}>
<CardStatus />
</GridItem>
<GridItem span={4}>
<EventsCard />
</GridItem>
</Grid>
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that the cards don't get so narrow, can you use props to adjust the columns at narrow widths instead? The 3 across cards could stack when the screen is too narrow.
image

Copy link
Copy Markdown
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! I think this is fine to go.

Copy link
Copy Markdown
Collaborator

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this so far! It looks awesome!

A few comments/change requests:

const [shouldNotify, setShouldNotify] = useState(false);
const prevUnreadCountRef = useRef(0);

const getNumberUnread = () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to put this into a useCallback to prevent it from causing infinite rerenders since it's listed as a dep in some of your useEffects

Comment on lines +477 to +479
visibility={{
default: 'hidden'
}} /** this kebab dropdown replaces the icon buttons and is hidden for desktop sizes */
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With being set to hidden by default and no other visibility options won't this just never render?

<Masthead>
<MastheadMain>
<MastheadToggle>
<PageToggleButton variant="plain" aria-label="Global navigation" isHamburgerButton />
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing errors in the console related to the isHamburgerButton being passed here without an isExpanded prop.

);
};

const CardStatus: FunctionComponent = () => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having these components being defined from within another component is potentially problematic, and breaks the rules of hooks for the hooks you're using within them.

const onKebabDropdownSelect = () => setIsKebabDropdownOpen(false);
const onCloseNotificationDrawer = (_event: any) => setIsDrawerExpanded((prevState) => !prevState);

useEffect(() => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also just on a more general note my gut says that the way that all of these alerts are being made and managed could be DRY'd up substantially, but since this is just a demo I'm not too worried about that.

@parthivrh parthivrh force-pushed the animation-demo-page branch from a080916 to ae41f60 Compare July 17, 2025 17:13
@edonehoo
Copy link
Copy Markdown
Contributor

When you get a chance, could you also incorporate the content updates I snuck into Jeff's PR? #11915

Open to other ideas, but I feel like the page content might be better as more of an intro, rather than "docs", since we moved to the guided tour

@nicolethoen
Copy link
Copy Markdown
Contributor

Closing this because all of the changes here have been integrated into: #11915 and we are picking up in that PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Animation - create demo/showcase page

7 participants